python/packages/jumpstarter-driver-mitmproxy/examples/addons/_template.py-95-99 (1)

95-99: ⚠️ Potential issue | 🟡 Minor

messages[-1] raises IndexError when the messages list is empty.

The guard on line 92 only checks for None, but flow.websocket.messages can be an empty list (e.g. on connection open before any message arrives), making the index access on line 95 blow up. Since this template is copied verbatim by users, the bug will propagate.

🛡️ Proposed fix

-        last_msg = flow.websocket.messages[-1]
-        if last_msg.from_client:
+        if not flow.websocket.messages:
+            return
+        last_msg = flow.websocket.messages[-1]
+        if last_msg.from_client:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/addons/_template.py`
around lines 95 - 99, The code indexes flow.websocket.messages[-1] without
ensuring the list is non-empty, which raises IndexError when messages == [];
update the guard around that block (in the area using last_msg and
flow.websocket.messages) to check that flow.websocket.messages is not None and
has at least one element (e.g. check truthiness or len(flow.websocket.messages)
> 0) before assigning last_msg and calling ctx.log.info so you only access the
last element when the list contains an item.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/_template.py-1-33 (1)

1-33: ⚠️ Potential issue | 🟡 Minor

Module docstring and example snippet reference "JSON" but the driver uses a YAML-based scenario system.

Lines 6, 11–16, and 56–57 all say "scenario JSON" / "mock scenario JSON". The PR description explicitly states this is a "YAML-based scenario system", and the scenario examples use YAML. Users copying this template will be confused about which format to use for addon / addon_config.

📝 Proposed fix

-The filename (without .py) becomes the addon name you reference
-in mock scenario JSON files.
+The filename (without .py) becomes the addon name you reference
+in mock scenario YAML files.

-Example: Save as ``addons/my_custom_api.py``, then reference in
-your scenario JSON as::
+Example: Save as ``addons/my_custom_api.py``, then reference in
+your scenario YAML as::

-    "GET /my/endpoint/*": {
-        "addon": "my_custom_api",
-        "addon_config": {
-            "any_key": "passed to your handler"
-        }
-    }
+    GET /my/endpoint/*:
+      addon: my_custom_api
+      addon_config:
+        any_key: passed to your handler

Also update lines 56–57 in the handle docstring:

-            config: The "addon_config" dict from the mock scenario
-                    JSON. Empty dict if not specified.
+            config: The "addon_config" dict from the mock scenario
+                    YAML. Empty dict if not specified.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/addons/_template.py`
around lines 1 - 33, The module docstring and examples incorrectly refer to a
"scenario JSON" system; update all occurrences that mention "JSON" or "mock
scenario JSON" to "YAML" (or "scenario YAML") so users know the driver uses a
YAML-based scenario system; update the example snippet that shows how to
reference the addon (the section that references "addon" and "addon_config") and
the handle(flow, config) docstring lines (currently mentioning JSON) to mention
YAML instead, ensuring the Handler class docstring and example usage
consistently reference YAML-based scenarios.

python/packages/jumpstarter-driver-mitmproxy/examples/scenarios/backend-degraded.yaml-41-45 (1)

41-45: ⚠️ Potential issue | 🟡 Minor

Misleading comment — 503 is not a timeout.

The comment says "time out" but the mock returns a 503 Service Unavailable after 8 s of latency. A timeout is a transport-layer event (no response); a 503 is an explicit server-side error. Consider updating the comment to accurately describe what is being simulated (slow 503) and whether it additionally covers timeout scenarios.

✏️ Suggested fix

-  # All telemetry uploads time out -- tests retry logic
+  # All telemetry uploads return 503 with high latency -- tests slow-failure retry logic
   POST /api/v1/telemetry/upload:
     status: 503
     body: {error: Backend overloaded}
     latency_ms: 8000

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/scenarios/backend-degraded.yaml`
around lines 41 - 45, Update the misleading comment above the POST
/api/v1/telemetry/upload mock: it currently says "time out" but the mock returns
status: 503 with latency_ms: 8000 (a slow 503), so change the comment to
indicate this simulates a slow server error (e.g., "All telemetry uploads return
a slow 503 to exercise retry logic"); if you intended to simulate
transport-layer timeouts as well, add a separate scenario that omits a response
or configures a transport timeout instead of using status: 503.

python/packages/jumpstarter-driver-mitmproxy/examples/exporter.yaml-1-35 (1)

1-35: ⚠️ Potential issue | 🟡 Minor

File is missing apiVersion, kind, and metadata — invalid standalone ExporterConfig.

The comment on Line 3 says to install this as /etc/jumpstarter/exporters/my-bench.yaml, but unlike demo/exporter.yaml this file has no top-level Jumpstarter metadata. Without those fields the Jumpstarter CLI will likely reject or misparse it.

🛠️ Proposed fix — add missing top-level headers

 # Example Jumpstarter exporter configuration with mitmproxy integration.
 #
 # Install to: /etc/jumpstarter/exporters/my-bench.yaml
 # ...
+apiVersion: jumpstarter.dev/v1alpha1
+kind: ExporterConfig
+metadata:
+  namespace: default
+  name: my-bench
+
 export:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/exporter.yaml` around
lines 1 - 35, This YAML lacks the top-level Jumpstarter resource headers; add
apiVersion, kind (ExporterConfig), and metadata.name at the top so the file
becomes a valid standalone ExporterConfig that the CLI will accept; keep the
existing top-level "export:" block and existing resources like the dutlink
(type: jumpstarter_driver_dutlink.driver.Dutlink) unchanged and follow the same
header values/structure used in demo/exporter.yaml for consistency.

python/packages/jumpstarter-driver-mitmproxy/pyproject.toml-23-25 (1)

23-25: ⚠️ Potential issue | 🟡 Minor

Replace repo with jumpstarter in the source_archive URL.

The source_archive URL currently uses https://github.com/jumpstarter-dev/repo/archive/{commit_hash}.zip, but the actual repository is jumpstarter-dev/jumpstarter. Update to https://github.com/jumpstarter-dev/jumpstarter/archive/{commit_hash}.zip.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/pyproject.toml` around lines 23
- 25, Update the source_archive entry in pyproject.toml to point to the correct
GitHub repository: change the value of the source_archive key from using "repo"
to "jumpstarter" so it becomes
"https://github.com/jumpstarter-dev/jumpstarter/archive/{commit_hash}.zip"; edit
the source_archive line in the [tool.hatch.metadata.hooks.vcs.urls] section to
perform this replacement.

python/packages/jumpstarter-driver-mitmproxy/README.md-379-388 (1)

379-388: ⚠️ Potential issue | 🟡 Minor

Missing Dockerfile for documented container deployment.

The README's "Container Deployment" section (lines 379-388) instructs users to run podman build -t jumpstarter-mitmproxy:latest ., but no Dockerfile or Containerfile exists in the package directory. Users following these instructions will encounter an immediate build error. Either provide the container build artifact or remove this deployment section from the README.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/README.md` around lines 379 -
388, The README's "Container Deployment" section references building a container
with the command "podman build -t jumpstarter-mitmproxy:latest ." and running
"jmp exporter start my-bench" but there is no Dockerfile/Containerfile in the
package, causing build failures; either add a Dockerfile/Containerfile that
reproduces the image referenced by the podman build command (ensuring it exposes
ports 8080/8081, mounts /etc/jumpstarter as shown, and installs the jmp CLI and
exporter entrypoint), or remove/update the "Container Deployment" snippet in
README.md (the podman build/run lines and the "jmp exporter start my-bench"
example) to avoid instructing users to build a non-existent image.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/hls_audio_stream.py-51-67 (1)

51-67: ⚠️ Potential issue | 🟡 Minor

ADTS frame header encodes frame_length = 0 — malformed frame that strict decoders will reject.

The aac_frame_length field is a non-byte-aligned 13-bit field spanning bytes 3–5 of the ADTS header:

• byte 3 [1:0] → frame_length[12:11]
• byte 4 [7:0] → frame_length[10:3]
• byte 5 [7:5] → frame_length[2:0]

For a 31-byte frame (7-byte header + 24 bytes of silence data) the correct values are:

• byte 4 = 0x03 (bits 10–3 of 31 = 0b00000011)
• byte 5 = 0xFF (bits 2–0 of 31 = 0b111 in positions 7–5; VBR buffer-fullness 0x7FF fills bits 4–0 = 0b11111)

The current \x00\x1f packs 31 as a plain big-endian 16-bit integer, ignoring the bit offsets. As a result, the aac_frame_length field decodes to 0, making the frame technically invalid per ISO 14496-3 ADTS. Strict decoders (and DUT audio stacks under test) may skip or error on every generated frame, defeating the stated goal of exercising the DUT's decode/buffer/playback path.

🛠️ Proposed fix — correct byte 4 and byte 5

 SILENT_AAC_FRAME = (
     b"\xff\xf1"          # ADTS sync word + MPEG-4, Layer 0
     b"\x50"              # AAC-LC, 44100 Hz (idx 4)
     b"\x80"              # Channel config: 2 (stereo)
-    b"\x00\x1f"          # Frame length (header + padding)
-    b"\xfc"              # Buffer fullness (VBR)
+    b"\x03\xff"          # frame_length=31: byte4=frame_len[10:3]=0x03, byte5=frame_len[2:0]=0b111<<5 | buf_full[10:6]=0b11111
+    b"\xfc"              # Buffer fullness (VBR): buf_full[5:0]=0b111111, num_blocks=0b00
     + b"\x00" * 24       # Silent spectral data
 )

Derivation: frame = 7 (header) + 24 (data) = 31 = 0b0000000011111

• byte 3 [1:0] = bits[12:11] = 00 → 0x80 unchanged ✓
• byte 4 = bits[10:3] = 0b00000011 = 0x03
• byte 5 [7:5] = bits[2:0] = 0b111; [4:0] = VBR (0b11111) → 0b11111111 = 0xFF
• byte 6 = VBR bits[5:0] + num_blocks → 0b11111100 = 0xFC ✓

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/addons/hls_audio_stream.py`
around lines 51 - 67, SILENT_AAC_FRAME currently encodes the 13-bit ADTS
aac_frame_length incorrectly (producing 0); update the header bytes in
SILENT_AAC_FRAME so bytes 3–5 contain the properly bit-packed frame_length for a
31-byte frame (7-byte header + 24 bytes payload): change the sequence that is
currently b"\x00\x1f" b"\xfc" to use b"\x03\xff" b"\xfc" (i.e., set byte 4 =
0x03 and byte 5 = 0xFF) inside SILENT_AAC_FRAME, and if you ever change the
silent payload size update the aac_frame_length packing in SILENT_AAC_FRAME
accordingly (or compute it programmatically in _generate_silent_segment).

python/packages/jumpstarter-driver-mitmproxy/examples/scenarios/update-available.yaml-33-38 (1)

33-38: ⚠️ Potential issue | 🟡 Minor

GET /api/v1/status appears truncated — missing fields present in the demo variant.

Compared to demo/scenarios/update-available.yaml, this endpoint is missing firmware_version, uptime_s, and source. Since this lives under examples/, users will reference it as a template. Either complete it or add a comment indicating it's intentionally minimal.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/scenarios/update-available.yaml`
around lines 33 - 38, The GET /api/v1/status example response is missing fields
compared to the demo variant; update the example scenario for the GET
/api/v1/status endpoint to include the missing keys (firmware_version, uptime_s,
source) with representative values, or alternatively add a clear YAML comment
above the response stating this is intentionally minimal and not a full
template; target the GET /api/v1/status block in
examples/scenarios/update-available.yaml (the endpoint name and body keys) when
making the change.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/mjpeg_stream.py-244-261 (1)

244-261: ⚠️ Potential issue | 🟡 Minor

Fix lint failures: unused frame_interval and loop variable i.

Pipeline is failing due to these lint issues. frame_interval is assigned but never used (F841), and i in the loop is unused (B007).

Proposed fix

-        frame_interval = 1.0 / fps
         burst_duration_s = 10  # Generate 10 seconds of frames
         num_frames = int(burst_duration_s * fps)
 
         parts = []
-        for i in range(num_frames):
+        for _ in range(num_frames):
             frame = self._get_frame(
                 camera_id, frames_dir, resolution,
             )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/addons/mjpeg_stream.py`
around lines 244 - 261, Remove the unused variables to satisfy linters: delete
the unused assignment to frame_interval and change the loop to use a throwaway
variable; specifically, remove the line setting frame_interval = 1.0 / fps and
update the loop "for i in range(num_frames):" to "for _ in range(num_frames):"
while keeping the existing body that calls self._get_frame(camera_id,
frames_dir, resolution) and appends to parts (boundary, num_frames, parts, and
_get_frame are the relevant symbols).

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py-370-388 (1)

370-388: ⚠️ Potential issue | 🟡 Minor

restart() cannot explicitly disable web_ui.

Line 381: restart_web = web_ui or self._web_ui_enabled — passing web_ui=False explicitly will still pick up self._web_ui_enabled as True if the previous run had the web UI enabled. There's no way for a caller to turn it off on restart.

Consider using a tri-state (e.g., web_ui: bool | None = None) where None means "keep previous" and False means "explicitly disable".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 370 - 388, The restart method currently can't disable the web UI
because restart_web = web_ui or self._web_ui_enabled always falls back to the
previous state; change the restart signature to accept a tri-state for web_ui
(e.g., web_ui: bool | None = None) and update logic so that if web_ui is None
you keep self._web_ui_enabled, otherwise use the explicit boolean value; update
the restart() body to compute restart_web = self._web_ui_enabled if web_ui is
None else web_ui and ensure the call into start(restart_mode, restart_web,
replay_file) and any type hints/use sites are adjusted accordingly.

python/packages/jumpstarter-driver-mitmproxy/demo/test_demo.py-252-276 (1)

252-276: ⚠️ Potential issue | 🟡 Minor

test_assert_request_made_with_wildcard also relies on uncleaned capture state.

Same issue as test_wait_for_request — without clearing captured requests, the assert_request_made("GET", "/api/v1/*") wildcard could match stale requests from earlier tests (e.g., GET /api/v1/status from test_capture_context_manager).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/demo/test_demo.py` around lines
252 - 276, The test leaves previous captured requests in proxy_client's state so
the wildcard assertion can match stale entries; before running the wildcard
assertion in test_assert_request_made_with_wildcard, clear the proxy client's
captured request buffer (e.g. call the
proxy_client.clear_captured_requests()/clear_requests() method or equivalent)
after wait_for_request() and before proxy_client.assert_request_made("GET",
"/api/v1/*") so only requests made in this test are considered; ensure you use
the existing proxy_client API (methods seen: set_mock, wait_for_request,
assert_request_made) to locate where to insert the clear call.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/data_stream_websocket.py-235-267 (1)

235-267: ⚠️ Potential issue | 🟡 Minor

dt is hardcoded to 0.1 s, but the actual push interval is configurable.

dt on Line 237 drives battery drain, counter accumulation, GPS drift, and temperature rise, yet it ignores the real interval_s. When push_interval_ms is set to e.g. 500 ms, the physics simulation will run at 5× the intended rate per wall-clock second.

Proposed fix

Pass the actual interval into SensorState (or next_frame) instead of hardcoding:

 class SensorState:
-    def __init__(self, scenario: dict):
+    def __init__(self, scenario: dict, dt: float = 0.1):
         self.scenario = scenario
         self.t0 = time.time()
         self.frame_num = 0
+        self.dt = dt
         ...

     def next_frame(self) -> dict:
-        dt = 0.1  # ~100ms per frame
+        dt = self.dt
         elapsed = time.time() - self.t0

And in _push_telemetry:

-        state = SensorState(scenario)
+        state = SensorState(scenario, dt=interval_s)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/addons/data_stream_websocket.py`
around lines 235 - 267, The physics timestep dt is hardcoded to 0.1s inside
SensorState.next_frame(), causing state updates to ignore the configurable
push_interval_ms; change next_frame to accept an interval_s parameter (or set an
instance attribute on SensorState from _push_telemetry) and use that value
instead of the hardcoded dt for battery drain, counter accumulation, GPS drift,
temperature, and any other time-based updates. Update all callers (e.g.,
_push_telemetry) to compute interval_s = push_interval_ms / 1000.0 and pass it
into SensorState.next_frame (or set SensorState.interval_s) so the simulation
runs with the real push interval. Ensure fallback behavior if interval_s is
missing (e.g., default to previous value) to avoid breaking tests.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/_template.py (1)

30-33: cleanup() is documented in the module docstring but absent from the class.

The docstring describes cleanup() as a reserved lifecycle hook (lines 30–33), but the Handler class provides no stub for it. Users copying this template won't know where to add cleanup logic and may miss implementing it when the hook becomes active.

✨ Proposed addition

     def websocket_message(self, flow: http.HTTPFlow, config: dict):
         ...

+    def cleanup(self):
+        """Called when the addon is unloaded (reserved for future use)."""
+        pass

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/addons/_template.py`
around lines 30 - 33, The module docstring mentions a lifecycle hook cleanup()
but the Handler class lacks this stub; add a cleanup(self) method to the Handler
class (matching the other lifecycle methods' signature/style) that contains a
short docstring stating it's a reserved/no-op hook and a no-op body so users
know where to put teardown logic when the hook becomes active; ensure the method
name is exactly cleanup to match the docstring and follow the same
visibility/formatting as the existing methods.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/hls_audio_stream.py (1)

80-81: _segment_cache is initialized but never read or written — dead code.

♻️ Proposed fix

 def __init__(self):
     self._sequence_counters: dict[str, int] = {}
-    self._segment_cache: bytes | None = None

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/addons/hls_audio_stream.py`
around lines 80 - 81, The field _segment_cache on the class is dead code (never
read or written); remove the unused attribute declaration or implement proper
caching where segments are processed. Locate the class that defines
self._segment_cache (the __init__ or attribute block with
self._sequence_counters and self._segment_cache) and either delete the
self._segment_cache: bytes | None = None line, or if intended, add reads/writes
to it in the segment handling methods (e.g., the HLS segment fetch/serve
functions) and ensure tests cover the caching behavior.

python/packages/jumpstarter-driver-mitmproxy/demo/dut_simulator.py (1)

85-85: Prefer urllib.parse.urlsplit over string-splitting for URL path extraction.

url.split(backend)[-1] breaks if the backend host:port string appears anywhere in the path. Using urllib.parse.urlsplit(url).path is both more robust and idiomatic.

♻️ Proposed refactor

+from urllib.parse import urlsplit
+
 def run_cycle(session: requests.Session, backend: str):
     ...
     for method, url in endpoints:
-        path = url.split(backend)[-1]
+        path = urlsplit(url).path

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/demo/dut_simulator.py` at line
85, The current extraction uses path = url.split(backend)[-1], which fails if
the backend host:port appears elsewhere in the URL; replace this with using
urllib.parse.urlsplit(url).path to reliably get the path component (and add from
urllib.parse import urlsplit if not present). Update the assignment in the same
function/scope where path is set (referenced as path and url) to use
urlsplit(url).path so path extraction is robust and idiomatic.

python/packages/jumpstarter-driver-mitmproxy/pyproject.toml (1)

14-18: testpaths excludes demo/ — demo tests won't run by default.

testpaths = ["jumpstarter_driver_mitmproxy"] only runs the module-level test suite. Tests under demo/test_demo.py and examples/test_device.py are silently skipped. If those tests are only intended to run in a HiL environment, this is intentional and a comment here would clarify it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/pyproject.toml` around lines 14
- 18, The pytest configuration's testpaths only includes
"jumpstarter_driver_mitmproxy", which excludes demo/ and examples/ so
demo/test_demo.py and examples/test_device.py are not run; either add those
folders to the testpaths array (e.g., include "demo" and "examples") so they run
by default, or keep the current testpaths but add a concise inline comment in
the [tool.pytest.ini_options] section stating that demo/ and examples/ tests are
intentionally omitted because they require HiL (hardware-in-the-loop) to avoid
confusion; update the pyproject.toml entry for testpaths accordingly and ensure
the change references the testpaths setting.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py (1)

93-93: Class-level mutable _counters is shared across all TemplateEngine uses.

_counters is a class attribute (defaultdict(int)), meaning all callers share the same counter state. If there's only ever one TemplateEngine used as a classmethod-based singleton, this is fine — but if this module is ever loaded in multiple contexts (e.g., tests), counters will leak between them. If this is intentional, a brief comment would clarify the design choice.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py`
at line 93, The class-level mutable _counters (defaultdict(int)) on
TemplateEngine leaks state across uses; change it to be instance-scoped or
explicitly reset so each TemplateEngine gets its own counters (e.g., initialize
self._counters = defaultdict(int) in TemplateEngine.__init__ or lazy-initialize
per-instance), or if shared behavior is intentional add a clarifying comment
above the _counters declaration; update references to use self._counters and
remove reliance on the class attribute.

python/packages/jumpstarter-driver-mitmproxy/demo/conftest.py (2)

81-86: http_session only configures an HTTP proxy — HTTPS requests will bypass the proxy.

If any test exercises HTTPS endpoints, traffic will go direct rather than through mitmproxy. Consider adding the HTTPS proxy and disabling certificate verification (or trusting the mitmproxy CA) for full coverage.

♻️ Proposed enhancement

 def http_session():
     """Requests session pre-configured to route through the proxy."""
     session = requests.Session()
-    session.proxies = {"http": f"http://127.0.0.1:{PROXY_PORT}"}
+    session.proxies = {
+        "http": f"http://127.0.0.1:{PROXY_PORT}",
+        "https": f"http://127.0.0.1:{PROXY_PORT}",
+    }
+    session.verify = False  # Trust mitmproxy's CA for demo tests
     return session

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/demo/conftest.py` around lines
81 - 86, The http_session fixture currently only sets the "http" proxy so HTTPS
traffic bypasses mitmproxy; update the http_session fixture (requests.Session in
http_session) to also set "https": f"http://127.0.0.1:{PROXY_PORT}" in
session.proxies and either disable cert verification for tests (session.verify =
False) or configure the session to trust the mitmproxy CA, so HTTPS requests are
routed through and accepted by mitmproxy.

---

55-68: proxy fixture doesn't declare backend_server as a dependency.

If tests rely on the proxy forwarding to the backend in passthrough mode, the backend may not be started. If mock-mode tests never need the backend, this is fine — but it would be safer to add an explicit dependency or document the expectation.

♻️ Option: add backend_server dependency

 `@pytest.fixture`(scope="session")
-def proxy(client):
+def proxy(client, backend_server):

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/demo/conftest.py` around lines
55 - 68, The proxy fixture currently starts the mitmproxy driver but does not
declare backend_server as a pytest dependency, which can leave the backend
unstarted for tests that use passthrough; update the proxy fixture signature
(the pytest fixture named proxy) to accept backend_server as a parameter (or
explicitly call/start backend_server inside proxy) so the backend is ensured to
be running when proxy.start("mock") is invoked, or alternatively add a clear
docstring note in the proxy fixture indicating tests run in mock mode must not
rely on a backend; reference the proxy fixture and backend_server symbol to
locate where to add the dependency or documentation.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/client.py (1)

374-376: Redundant local import of Path.

Path is already imported at module level (line 42). The local import on line 375 is unnecessary.

Proposed fix

         def cert_cmd(output: str):
             ...
-            from pathlib import Path
-
             pem = self.get_ca_cert()
             out = Path(output)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/client.py`
around lines 374 - 376, Remove the redundant local import "from pathlib import
Path" inside the function/block and use the module-level Path that is already
imported at the top of the file (module import at line 42); locate the local
import statement in jumpstarter_driver_mitmproxy.client.py (the inner "from
pathlib import Path") and delete it so code continues referring to Path directly
without re-importing.

python/packages/jumpstarter-driver-mitmproxy/examples/conftest.py (1)

113-121: Docstring/name mismatch: mock_slow_backend returns 504 instantly.

The fixture name suggests simulating slowness, and the docstring says "gateway timeout", but it returns a 504 response immediately without any latency. For a slow-backend simulation, consider using set_mock_with_latency. As-is, this is functionally identical to a simple error mock. Since this is an example file, it may confuse users who adopt it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/conftest.py` around
lines 113 - 121, The fixture mock_slow_backend and its docstring claim to
simulate a slow backend but currently use proxy.mock_endpoint returning a 504
immediately; replace or update it to actually introduce latency (e.g., use
proxy.set_mock_with_latency or the equivalent API) so the endpoint both delays
the response and then returns a 504 (or update the name/docstring to reflect an
immediate 504 if you intend to keep no latency). Locate the mock_slow_backend
fixture and change the proxy.mock_endpoint usage to the latency-aware helper
(set_mock_with_latency) with the same method/path and status/body, or rename the
fixture/docstring to indicate an immediate 504 error.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver_integration_test.py (1)

134-142: Minor flakiness risk: no _wait_for_port before status check in passthrough test.

test_start_passthrough_mode doesn't call _wait_for_port after client.start("passthrough"), unlike other tests. If status() is called before mitmdump is fully up, running could still be True (based on PID), but the test style is inconsistent with the rest of the suite.

Suggested fix

     def test_start_passthrough_mode(self, client):
         result = client.start("passthrough")
         assert "passthrough" in result
+        # Wait for proxy to be ready (consistent with other tests)

         status = client.status()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver_integration_test.py`
around lines 134 - 142, In test_start_passthrough_mode add the same readiness
wait used elsewhere: after calling client.start("passthrough") call the helper
_wait_for_port (or the test-suite's equivalent readiness helper) before invoking
client.status(); this ensures mitmdump is fully accepting connections so
status() reflects the running/ready state consistently—update
test_start_passthrough_mode to call _wait_for_port (or the existing wait helper)
between client.start and client.status and keep client.stop() as-is.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/mjpeg_stream.py (1)

294-296: Accept files_base from addon config instead of hardcoding the path.

/opt/jumpstarter/mitmproxy/mock-files is hardcoded rather than being derived from addon config. Since the handle() method already receives a config dict and extracts other parameters like frames_dir, fps, and resolution, the base path should also be configurable. This would allow different deployments to override the mock-files directory.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/addons/mjpeg_stream.py`
around lines 294 - 296, The code currently hardcodes files_base =
Path("/opt/jumpstarter/mitmproxy/mock-files") inside the addon's handle()
method; change it to read files_base (or mock_files_base) from the addon config
dictionary alongside frames_dir/fps/resolution, defaulting to the current path
if not provided, and then use that variable when constructing frame_dir =
files_base / frames_dir so deployments can override the mock-files directory via
config.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py (3)

52-63: Default bind address 0.0.0.0 exposes the proxy and web UI to all network interfaces.

Both ListenConfig and WebConfig default to 0.0.0.0, which exposes the mitmproxy listener and the mitmweb UI (with its default password) to the network. For a proxy that intercepts HTTPS traffic, this broadens the attack surface unnecessarily. Consider defaulting to 127.0.0.1 and letting users opt in to broader binding.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 52 - 63, Defaulting ListenConfig.host and WebConfig.host to
"0.0.0.0" exposes mitmproxy and mitmweb to all network interfaces; change the
defaults in the ListenConfig and WebConfig classes so host defaults to
"127.0.0.1" instead, and update any related docs/tests that assume the old
default to make it explicit that users can opt in to broader binding if needed
(search for ListenConfig and WebConfig to locate usages).

---

1180-1194: String replacement to patch paths in bundled addon is brittle.

str.replace() will substitute every occurrence of the default path strings in the bundled addon source, not just the intended configuration constants. If those strings appear in comments, docstrings, or error messages, they'll be silently corrupted.

A safer approach: use sentinel markers (e.g. __MOCK_DIR_PLACEHOLDER__) in the bundled addon that are unlikely to appear elsewhere, or write a small JSON sidecar config that the addon reads at startup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 1180 - 1194, The current approach mutates bundled_addon.py by blind
string replacement (see variables bundled, path, self.directories.mocks, and
self._capture_socket_path) which can corrupt unintended text; instead modify the
bundled addon to use distinct sentinel placeholders (e.g.
__MOCK_DIR_PLACEHOLDER__ and __CAPTURE_SOCKET_PLACEHOLDER__) or load a small
JSON sidecar config at runtime, then update this driver to only replace those
sentinel tokens (or write the sidecar file next to the addon) so replacements
are deterministic and cannot affect comments/docstrings or other literals.

---

1023-1037: Accept loop overwrites _capture_reader_thread — only the last connection is joinable on shutdown.

If the addon reconnects (or multiple addons connect), each accepted connection spawns a new thread (Line 1028-1033), but only the latest reference is stored in self._capture_reader_thread. Previous reader threads become unjoinable, leaking thread objects on _stop_capture_server.

Consider storing reader threads in a list and joining them all on shutdown.

Proposed fix

-    _capture_reader_thread: threading.Thread | None = field(
-        default=None, init=False, repr=False
-    )
+    _capture_reader_threads: list = field(
+        default_factory=list, init=False, repr=False
+    )

In _capture_accept_loop:

-                self._capture_reader_thread = threading.Thread(
+                t = threading.Thread(
                     target=self._capture_read_loop,
                     args=(conn,),
                     daemon=True,
                 )
-                self._capture_reader_thread.start()
+                t.start()
+                self._capture_reader_threads.append(t)

In _stop_capture_server:

-        if self._capture_reader_thread is not None:
-            self._capture_reader_thread.join(timeout=5)
-            self._capture_reader_thread = None
+        for t in self._capture_reader_threads:
+            t.join(timeout=5)
+        self._capture_reader_threads.clear()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 1023 - 1037, The accept loop in _capture_accept_loop stores each
new reader thread only in self._capture_reader_thread, so earlier threads cannot
be joined and leak; change this to append each new Thread to a list attribute
(e.g., self._capture_reader_threads) when created, ensure the list is
initialized where the server starts, and in _stop_capture_server iterate over
and join all threads from self._capture_reader_threads (optionally with a
timeout) and clear the list; if concurrent access is possible, protect
append/iterate with the same lock used for server state.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/data_stream_websocket.py (1)

155-177: Cancelled tasks are not removed from _tasks on set_scenario.

On unsubscribe/stop (Line 158) the entry is deleted, but set_scenario (Line 165) cancels without deleting before overwriting. This is functionally fine since Line 176 overwrites the entry, but there's a subtler gap: if the old task's CancelledError handler (Line 202-203) runs after the new task is stored, there's no conflict — though for consistency you could del before reassigning.

More importantly, if a flow disconnects without an unsubscribe, the _tasks entry and the completed asyncio.Task object are never cleaned up (the entry lingers forever). For a long-lived exporter this is a slow memory leak. Consider cleaning up in _push_telemetry's finally block.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/addons/data_stream_websocket.py`
around lines 155 - 177, The problem: cancelled or completed asyncio.Tasks stored
in self._tasks (set_scenario handler and on disconnect) are never removed,
causing a memory leak; fix by ensuring tasks are deleted from self._tasks when
they are cancelled or finish. Update the set_scenario branch in the message
handler to delete the old task entry (del self._tasks[flow_id]) before
creating/assigning the new task, and modify _push_telemetry to remove its own
entry from self._tasks in a finally block (or on CancelledError/normal exit)
using the flow's id so the task is cleaned up when the websocket disconnects or
the task completes.

python/packages/jumpstarter-driver-mitmproxy/demo/test_demo.py (1)

70-132: Pervasive time.sleep(1) for mock hot-reload is fragile and slow.

Every test that sets a mock follows with time.sleep(1) to wait for the addon's file-mtime polling. This makes the suite slow (≥6 s of pure sleep) and racy on busy CI where file-system timestamps or scheduling can lag.

Consider adding a polling helper that verifies the mock is active (e.g., a lightweight sentinel request) rather than relying on a fixed sleep. Even in demo code, this would make the tests more reliable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/demo/test_demo.py` around lines
70 - 132, Replace the fragile time.sleep(1) calls with a polling helper that
actively verifies mock activation/removal: implement a helper (e.g.,
wait_for_mock_state(method, path, http_session, *, expect_active=True,
timeout=5, interval=0.1)) that repeatedly issues the sentinel request
(http_session.get("http://example.com" + path)) until the response matches the
expected state (status/body for active or a non-mock response for removed) or
times out; call this helper after proxy_client.set_mock and inside/after
proxy_client.mock_endpoint to wait for activation and after context exit to wait
for removal, and remove all time.sleep calls in tests like
test_mock_replaces_real_response, test_mock_error_status, and
test_mock_endpoint_context_manager.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py (4)

390-402: files_dir initialized with .. relative path — fragile default.

Line 393 sets self.files_dir = Path(self.MOCK_DIR) / "../mock-files", which produces an un-resolved path containing ... This works at runtime because _load_config overwrites it from the config file, and _read_file calls .resolve(). However, if _load_config fails or config has no files_dir, the .. stays in the path, and the path traversal check in _read_file (which compares resolved paths) may behave unexpectedly depending on whether MOCK_DIR itself contains symlinks.

Use .parent / "mock-files" instead of / "../mock-files" for clarity and to avoid resolution surprises.

Proposed fix

-        self.files_dir: Path = Path(self.MOCK_DIR) / "../mock-files"
+        self.files_dir: Path = Path(self.MOCK_DIR).parent / "mock-files"

Also apply at Line 434:

-                self.files_dir = Path(self.MOCK_DIR).parent / "mock-files"
+                self.files_dir = Path(self.MOCK_DIR).parent / "mock-files"

(Line 434 already uses .parent — only Line 393 needs fixing.)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py`
around lines 390 - 402, The default files_dir in __init__ is set using a "../"
relative path which can leave unresolved ".." components and cause the
path-traversal check in _read_file to behave unpredictably; change the
initialization of self.files_dir in __init__ to construct the directory from
Path(self.MOCK_DIR).parent / "mock-files" (same style already used elsewhere) so
the path has the correct parent-based form before _load_config or _read_file
run, leaving _load_config and _read_file unchanged.

---

326-340: CaptureClient.send_event silently drops events on persistent socket failures.

If the Unix socket is unavailable (driver not running or socket path mismatch), all capture events are silently lost. This is documented behavior ("events are silently dropped"), but for debugging test failures it can be confusing when wait_for_request times out with no indication that the capture pipeline is broken. Consider logging a warning on the first failed connection attempt.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py`
around lines 326 - 340, send_event currently drops events silently when the Unix
socket is unavailable; modify CaptureClient.send_event to log a warning on the
first failed connection attempt so failures are visible during tests: when
_connect() returns False or when the first OSError is caught, emit a single
warning (using the instance logger or module logger) that includes the socket
path and that events will be dropped, then proceed with the existing
reconnect/return logic; reference the send_event method, the _connect() call,
the self._sock check, and the close() call to locate where to insert the
warning.

---

619-653: request hook: _find_endpoint calls _load_config() on every request, plus _load_state() at Line 621.

Both _load_config and _load_state perform a stat() syscall per invocation to check file modification time. In a test environment this is acceptable, but for higher-traffic scenarios the two stat calls per request could become a bottleneck. A short cooldown (e.g., only check every N seconds) would reduce the overhead without meaningfully delaying config hot-reload.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py`
around lines 619 - 653, The request hook calls _find_endpoint (which calls
_load_config()) and also calls _load_state() on every request causing two stat()
syscalls per request; add a short cooldown to both _load_config and _load_state
so they only perform their file mtime stat when enough seconds have passed since
the last check. Implement per-instance fields like _last_config_check,
_last_state_check and a configurable _config_check_interval (seconds); on first
call always load, otherwise compare time.time() to last check and skip the
stat/syscall if inside the cooldown window, updating the last check time and
reloading only when the interval has elapsed so request(), _find_endpoint(),
_load_config(), and _load_state() retain hot-reload behavior without calling
stat() every request.

---

302-302: CAPTURE_SOCKET is a hardcoded module-level constant but the driver generates dynamic socket paths.

The driver's _start_capture_server picks a socket path dynamically (preferred path or temp file), then patches this constant via string replacement in _generate_default_addon. If the bundled addon is loaded outside the driver's copy mechanism (e.g., directly via mitmdump -s bundled_addon.py), it will use the hardcoded /opt/jumpstarter/mitmproxy/capture.sock path which may not match the driver's actual socket. Consider reading the socket path from an environment variable as a fallback to decouple the addon from the driver's patching step.

Proposed fix

-CAPTURE_SOCKET = "/opt/jumpstarter/mitmproxy/capture.sock"
+CAPTURE_SOCKET = os.environ.get(
+    "JMP_CAPTURE_SOCKET", "/opt/jumpstarter/mitmproxy/capture.sock"
+)

Then in the driver's _generate_default_addon, set the env var instead of (or in addition to) string-patching.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py`
at line 302, The module-level CAPTURE_SOCKET constant is hardcoded which breaks
when the addon is run standalone; change the addon to read the socket path from
an environment variable fallback (e.g.,
os.environ.get("JUMPSTARTER_CAPTURE_SOCKET",
"/opt/jumpstarter/mitmproxy/capture.sock")) instead of relying solely on the
literal CAPTURE_SOCKET, and update the driver's _generate_default_addon to set
that environment variable (or continue patching but prefer setting
JUMPSTARTER_CAPTURE_SOCKET) so _start_capture_server and other functions use the
env var-derived path when present.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py (3)

1149-1171: Non-atomic write to endpoints.json — addon may read a partial file.

_write_mock_config and _write_state open the target file and write directly. If the mitmproxy addon hot-reloads at the same moment, it could read truncated JSON. The addon's _load_config catches the exception and skips the update, so this won't crash, but it silently drops a config change until the next trigger.

Write to a temp file in the same directory and os.replace() for an atomic swap.

Proposed fix for _write_mock_config

     def _write_mock_config(self):
         """Write mock endpoint definitions to disk in v2 format."""
         mock_path = Path(self.directories.mocks)
         mock_path.mkdir(parents=True, exist_ok=True)
         config_file = mock_path / "endpoints.json"

         v2_config = {
             "config": {
                 "files_dir": self.directories.files,
                 "addons_dir": self.directories.addons,
                 "default_latency_ms": 0,
                 "default_content_type": "application/json",
             },
             "endpoints": self._mock_endpoints,
         }

-        with open(config_file, "w") as f:
-            json.dump(v2_config, f, indent=2)
+        tmp_file = config_file.with_suffix(".json.tmp")
+        with open(tmp_file, "w") as f:
+            json.dump(v2_config, f, indent=2)
+        os.replace(tmp_file, config_file)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 1149 - 1171, Replace the non-atomic direct write in
_write_mock_config by writing the JSON to a temp file in the same directory (use
tempfile.NamedTemporaryFile or mkstemp with dir=mock_path and delete=False),
flush and fsync the temp file, close it, then atomically swap it into place with
os.replace(temp_path, config_file); also apply the same pattern to _write_state
so both writes are atomic; ensure you import os and tempfile and maintain the
same file permissions/ownership if needed.

---

262-284: Hardcoded web UI password "jumpstarter" embedded in command args and status URLs.

Line 278 sets web_password=jumpstarter and Line 331 embeds ?token=jumpstarter in the returned URL. While this is a dev/test tool, a fixed credential means anyone on the network can access the mitmweb UI. Consider making this configurable (e.g., a field on WebConfig) or generating a random token per session.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 262 - 284, The code currently hardcodes the mitmweb UI password
("jumpstarter") in _build_base_cmd and embeds the same token in the returned
status URL; change this to a configurable or per-session secret by adding a
web_password (or token) field to WebConfig (or generate a secure random token at
session start), then use that value when building the command in _build_base_cmd
(replace the literal "jumpstarter" with the config/token) and when composing the
status URL (replace the hardcoded ?token=jumpstarter); ensure the token is
generated with a cryptographically secure RNG, stored only in memory for the
session, and not logged.

---

1187-1297: Fallback addon generated via f-string interpolation of directory paths into Python source — fragile and injection-prone.

_generate_default_addon embeds self.directories.mocks directly into a Python source string. If the path contains characters like ", \, or {, the generated script will have syntax errors or unexpected behavior. The bundled addon path (Lines 1198-1214) is the primary path and uses string replacement which is slightly safer but still susceptible to the same issue. Since these are typically controlled server-side paths, the practical risk is low, but consider writing a small JSON config file alongside the addon instead of string-patching paths into source code.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 1187 - 1297, The fallback and bundled addon installation in
_generate_default_addon currently injects self.directories.mocks and
self._capture_socket_path directly into generated Python source (and does
string.replace on bundled_addon.py), which is fragile and injection-prone;
instead, write a small JSON config (e.g., {"mocks": "...", "capture_socket":
"..."}) next to the addon and change the addon to read that config at runtime
(referenced symbols: _generate_default_addon, bundled_addon.py,
MitmproxyMockAddon.MOCK_DIR, and self._capture_socket_path) — update the
bundled-copy branch to drop a config JSON rather than doing string.replace, and
change the inline addon template to load paths from that config file using
json.load at startup.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/mjpeg_stream.py (2)

294-294: Hardcoded base path /opt/jumpstarter/mitmproxy/mock-files is not configurable

The base files path is a magic constant baked into the method. It silently produces no frames (falling back to generated test patterns) if the addon is deployed at a different path, with no indication of why. Consider reading this from config or an environment variable so the example demonstrates the pattern correctly.

♻️ Suggested approach — read from config

-        files_base = Path("/opt/jumpstarter/mitmproxy/mock-files")
+        files_base = Path(config.get("mock_files_base", "/opt/jumpstarter/mitmproxy/mock-files"))

Note: config would need to be threaded through to _get_frame as a parameter if this approach is taken.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/addons/mjpeg_stream.py`
at line 294, The hardcoded files_base
Path("/opt/jumpstarter/mitmproxy/mock-files") should be made configurable: read
the base path from an injected config or an environment variable (e.g.,
os.environ["MJPEG_MOCK_FILES"] with a sensible default) and use that value
instead of the literal; if _get_frame is a helper method, add a parameter or
capture the config on the addon instance (e.g., self.files_base) so _get_frame
can reliably locate mock files and return a clear warning/log when the directory
is missing.

---

248-263: .encode() applied to adjacent f-string group — correct but visually misleading

Adjacent f-strings and regular strings are concatenated; regular strings at compile time and f-strings at run time. Each f-string is entirely evaluated before being concatenated to adjacent f-strings. Per Python's STRING+ grammar production, the four f-strings at lines 254-257 form one atom and .encode() is applied to the whole combined string — no TypeError at runtime. However, it appears visually as if only the final f"\r\n" is encoded, making the code easy to misread during maintenance. Use explicit parentheses to make the intent unambiguous.

♻️ Proposed fix — explicit grouping

         parts.append(
-            f"--{boundary}\r\n"
-            f"Content-Type: image/jpeg\r\n"
-            f"Content-Length: {len(frame)}\r\n"
-            f"\r\n".encode()
+            (
+                f"--{boundary}\r\n"
+                f"Content-Type: image/jpeg\r\n"
+                f"Content-Length: {len(frame)}\r\n"
+                f"\r\n"
+            ).encode()
             + frame
             + b"\r\n"
         )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/packages/jumpstarter-driver-mitmproxy/examples/addons/mjpeg_stream.py`
around lines 248 - 263, The header string build that is currently written as
adjacent f-strings with .encode() applied at the end (used when assembling parts
in the loop inside the mjpeg streamer) is visually misleading; update the header
construction in the loop that appends to parts so the entire header f-string
block is grouped explicitly (wrap the multi-line f-string sequence in
parentheses) before calling .encode(), e.g., build the header as one
parenthesized f-string (using boundary and len(frame)) and then .encode() +
frame + b"\r\n", ensuring the intent around .encode() on the full header is
unambiguous for future maintainers.

python/packages/jumpstarter-driver-mitmproxy/examples/addons/hls_audio_stream.py (2)

226-230: hash(resource) gives non-reproducible rotation across process restarts.

Python's string hash() is randomized by PYTHONHASHSEED, so the segment selected for a given resource name will differ between test runs. For a deterministic fallback rotation, use a stable hash such as hashlib.md5 or derive the index from the sequence number parsed out of resource.

♻️ Stable rotation using sequence number

+                # Parse the sequence number from "seg_{n}.aac" for stable rotation
+                try:
+                    seq_num = int(resource.removeprefix("seg_").removesuffix(".aac"))
+                    idx = seq_num % len(candidates)
+                except ValueError:
+                    idx = 0
-                idx = hash(resource) % len(candidates)
                 seg_path = candidates[idx]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/addons/hls_audio_stream.py`
around lines 226 - 230, The code uses Python's built-in hash(resource) in the
rotation logic (candidates, idx, seg_path), which is non-deterministic across
process restarts; replace it with a stable deterministic method such as
computing a stable hash (e.g., hashlib.md5(resource.encode()).hexdigest()
converted to an int) or derive the index from the segment sequence parsed from
the resource name, then compute idx = stable_int % len(candidates) and assign
seg_path = candidates[idx] so selection is reproducible across runs.

---

79-81: _sequence_counters and _segment_cache are never used — dead code.

Both fields are initialized but not read or written anywhere in the class. Either remove them or wire them up (e.g., use _segment_cache to avoid re-reading the same file on repeated requests, or use _sequence_counters for per-channel sequence tracking instead of the wall-clock derivation).

🧹 Remove unused fields

 def __init__(self):
-    self._sequence_counters: dict[str, int] = {}
-    self._segment_cache: bytes | None = None
+    pass

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/examples/addons/hls_audio_stream.py`
around lines 79 - 81, Remove the dead fields initialized in __init__: delete the
attributes self._sequence_counters and self._segment_cache (and their type
hints) since they are never read or written; if you prefer to keep functionality
instead, implement usage by wiring _segment_cache into the request handler to
cache and reuse a previously read audio segment and replace the wall-clock-based
sequence logic by using _sequence_counters per channel when building segment
URIs — otherwise simply remove these two attribute initializations from the
__init__ method and any related unused imports or references.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py (2)

95-95: _counters is a class-level singleton — counter state leaks across test runs.

TemplateEngine._counters is a class attribute (not instance), so {{counter(name)}} template expressions accumulate globally within a process. In pytest, counter values from one test will carry over into subsequent tests that use the same counter name, causing non-deterministic failures.

Consider adding a reset_counters() classmethod so test fixtures can call TemplateEngine.reset_counters() during teardown:

♻️ Proposed addition

     _counters: dict[str, int] = defaultdict(int)
+
+    `@classmethod`
+    def reset_counters(cls) -> None:
+        """Reset all counter state (call between tests for isolation)."""
+        cls._counters.clear()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py`
at line 95, TemplateEngine currently uses a class-level _counters: dict[str,int]
= defaultdict(int), causing counter state to persist across instances and tests;
add a classmethod reset_counters(cls) that clears or reinitializes cls._counters
(e.g., cls._counters.clear() or cls._counters = defaultdict(int)) and call that
from test fixtures as needed so {{counter(name)}} does not leak state between
test runs; update any places referring to TemplateEngine._counters to continue
using the class attribute but rely on reset_counters for lifecycle control.

---

328-334: Spool infrastructure is defined but never used — large/binary bodies are inlined unconditionally.

CAPTURE_SPOOL_DIR, _INLINE_BODY_LIMIT, _spool_dir, and _spool_counter are all defined but _build_capture_event (lines 895-910) always inlines the full request body via flow.request.get_text() or "", with no size check or spooling. For large uploads or binary content (e.g., firmware), this causes:

1. High memory pressure in the capture buffer.
2. get_text() decoding failures for non-UTF-8 binary bodies.

Either implement the spooling logic (checking against _INLINE_BODY_LIMIT and writing oversized/binary bodies to _spool_dir), or remove the dead constants and initialisation to keep the code clean.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/bundled_addon.py`
around lines 328 - 334, The spool constants (CAPTURE_SPOOL_DIR,
_INLINE_BODY_LIMIT, _spool_dir, _spool_counter) are unused while
_build_capture_event always inlines flow.request.get_text(); update
_build_capture_event to implement spooling: use flow.request.get_content() to
obtain bytes, detect non-UTF8/binary or check len(bytes) > _INLINE_BODY_LIMIT,
and when oversized/binary create _spool_dir if missing, write the bytes to a
unique file using _spool_counter (increment it safely), store the file path in
the capture event instead of inlining the body, and on normal small/UTF-8 bodies
decode to text (flow.request.get_text()) as before; include exception handling
to fall back to an empty inline body and log errors.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/client.py (1)

383-387: stop Event is never set — it's dead code.

The while not stop.wait(timeout=0.5) loop exits only via KeyboardInterrupt; stop is never set to True. The periodic timeout approach (to keep the GIL responsive) is correct, but stop adds confusion. Either document that stop is a placeholder for future external cancellation, or simplify:

♻️ Suggested simplification

-                    stop = Event()
-                    while not stop.wait(timeout=0.5):
-                        pass
+                    while True:
+                        time.sleep(0.5)

(and from threading import Event can be removed if no longer needed)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/client.py`
around lines 383 - 387, The local threading.Event named stop is never set and
makes the loop dead code; remove the Event usage and its import and simplify the
control flow by replacing the "stop" loop (while not stop.wait(timeout=0.5))
with an explicit loop that sleeps or waits for the timeout (e.g., while True:
time.sleep(0.5)) and keep the existing except KeyboardInterrupt: pass/break
behavior; update any surrounding comments to reflect that external cancellation
is not currently supported if needed.

python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py (1)

1313-1326: Fragile string-replacement coupling between driver.py and bundled_addon.py.

The two content.replace(...) calls depend on the replacement targets ('/opt/jumpstarter/mitmproxy/mock-responses' and '/opt/jumpstarter/mitmproxy/capture.sock') exactly matching MOCK_DIR's default value and CAPTURE_SOCKET in bundled_addon.py. If either constant is updated in one file but not the other, the replacement silently fails and the addon uses stale/wrong paths — a bug with no error signal.

Consider passing paths to the addon via environment variables or mitmproxy --set flags instead of post-copy string patching.

♻️ Sketch of env-var-based approach

In _build_base_cmd:

# Pass addon configuration as environment variables
env = os.environ.copy()
env["MITMPROXY_MOCK_DIR"] = self.directories.mocks
env["MITMPROXY_CAPTURE_SOCK"] = self._capture_socket_path or ""
self._subprocess_env = env  # store for Popen

In start():

self._process = subprocess.Popen(cmd, ..., env=self._subprocess_env)

Then bundled_addon.py reads these at startup, eliminating the string-replacement step entirely.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@python/packages/jumpstarter-driver-mitmproxy/jumpstarter_driver_mitmproxy/driver.py`
around lines 1313 - 1326, The current post-copy content.replace calls in
driver.py (replacing '/opt/jumpstarter/mitmproxy/mock-responses' and
'/opt/jumpstarter/mitmproxy/capture.sock') are fragile and should be removed;
instead pass the runtime paths into the addon via environment variables or
mitmproxy --set flags from _build_base_cmd so bundled_addon.py reads them at
startup. Modify _build_base_cmd to copy os.environ, set MITMPROXY_MOCK_DIR =
self.directories.mocks and MITMPROXY_CAPTURE_SOCK = self._capture_socket_path or
"", store that env on the instance (e.g. self._subprocess_env), and ensure
start() uses subprocess.Popen(cmd, ..., env=self._subprocess_env). Remove the
string-replacement block in the copy logic and update bundled_addon.py to read
os.environ['MITMPROXY_MOCK_DIR'] / ['MITMPROXY_CAPTURE_SOCK'] (or accept --set
values) so the addon no longer depends on brittle literal replacements.